Skip to content

Adds autoconfiguration for springboot apps.#459

Merged
artursouza merged 3 commits into
dapr:masterfrom
artursouza:springboot-autoconf
Jan 27, 2021
Merged

Adds autoconfiguration for springboot apps.#459
artursouza merged 3 commits into
dapr:masterfrom
artursouza:springboot-autoconf

Conversation

@artursouza

@artursouza artursouza commented Jan 27, 2021

Copy link
Copy Markdown
Contributor

Description

Adds autoconfiguration for springboot apps.

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #441

Inspired by #457

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@codecov

codecov Bot commented Jan 27, 2021

Copy link
Copy Markdown

Codecov Report

Merging #459 (677e93e) into master (efee056) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #459   +/-   ##
=========================================
  Coverage     82.77%   82.77%           
  Complexity      930      930           
=========================================
  Files            79       79           
  Lines          2717     2717           
  Branches        269      269           
=========================================
  Hits           2249     2249           
  Misses          334      334           
  Partials        134      134           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update efee056...911d3d0. Read the comment docs.

Comment thread sdk-tests/src/test/java/io/dapr/it/actors/app/TestApplication.java

@darren-wang darren-wang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it will work better with boot apps.

Comment thread sdk-springboot/pom.xml
Comment on lines +35 to +46
<dependencyManagement>
<dependencies>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-dependencies</artifactId>
<version>${springboot.version}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
</dependencies>
</dependencyManagement>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be careful of this segment, this will override okhttp version set in dapr-sdk, because spring boot also makes their version declaration in spring-boot-depencies, which conficts with dapr's.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this one I am OK since users would need to solve a version conflict too. If this becomes a problem we would know in our integration tests. Thanks for pointing this out. Learned one more thing.

Comment on lines +12 to +19
/**
* Dapr's Spring Boot AutoConfiguration.
*/
@Configuration
@ConditionalOnWebApplication
@ComponentScan("io.dapr.springboot")
public class DaprAutoConfiguration {
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use @ComponentScan here, also, you can replace it with @Bean method and @ConditionalOnProperty to provide finer grained control of beans, e.g. you can use property like dapr.controller.enabled to control whether to create the controller bean.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be a breaking change? If not, you could send a PR after this. You have more knowledge about spring boot than me.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we can release this version this time, and make additional improments in the future.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. That will also give you time to prepare a PR with your ideas. This one was great BTW. Thanks a lot.

@darren-wang darren-wang mentioned this pull request Jan 27, 2021
3 tasks
@artursouza artursouza merged commit 6c26942 into dapr:master Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Why not implement java-sdk-springboot as a spring boot starter?

3 participants